Conversation
…rong impure cache usage
|
Are comments supported for constants? I didn't see this in the documentation. Comments are simply supported for packaged procedures and functions. It would make sense to do the same for constants (and any package objects, for that matter). |
No, I didn't plan to implement comments, but I think it's possible. I will take a look |
Yes, that's exactly what I was asking about. Thanks for clarifying. |
|
None of IDs are persistent in Firebird and there is no reason for package's one to be the first. |
This isn't true for relations. That's why I asked. |
I've added permission checking connect employee user R;
Database: employee, User: R
SQL> select TEST.C1 from rdb$database;
Statement failed, SQLSTATE = 28000
no permission for USAGE access to PACKAGE "PUBLIC"."TEST"
-Effective user is RTo grant permissions: |
|
|
||
| FOR (REQUEST_HANDLE req_handle1) | ||
| CONST IN RDB$CONSTANTS | ||
| WITH CONST.RDB$SCHEMA_NAME NE SYSTEM_SCHEMA |
There was a problem hiding this comment.
Is this check allowed or should I add the RDB$SYSTEM_FLAG field?
… grant/revoke for constants in README
Noremos
left a comment
There was a problem hiding this comment.
I think this PR is ready to rereviw @AlexPeshkoff @asfernandes @dyemanov
src/isql/show.epp
Outdated
|
|
||
| bool isPrivate = !CONST.RDB$PRIVATE_FLAG.NULL && CONST.RDB$PRIVATE_FLAG; | ||
|
|
||
| if (!CONST.RDB$PACKAGE_NAME.NULL) |
There was a problem hiding this comment.
I removed the checks for now
src/jrd/obj.h
Outdated
| case obj_schemas: | ||
| return "SQL$SCHEMAS"; | ||
| case obj_package_constant: | ||
| return "SQL$PACKAGE_CONSTANT"; |
There was a problem hiding this comment.
I removed it for now
| class SysFunction | ||
| { | ||
| public: | ||
| enum Flags : UCHAR |
| GRANT SELECT ON TABLE secret TO PACKAGE pk_secret; | ||
| GRANT EXECUTE ON PACKAGE pk_secret TO ROLE role_secret; | ||
|
|
||
| To use package constants in a select expression, a USAGE permission is requeued: |
There was a problem hiding this comment.
| To use package constants in a select expression, a USAGE permission is requeued: | |
| To use package constants in a select expression, a USAGE permission is required: |
| the [<schema>.]<package>.<constant_name> notation. | ||
| Constants declared in the package body are private and cannot be accessed from outside the package. | ||
| However, they can be referenced directly by <constant_name> within <procedure_impl> and <function_impl>. | ||
| Header constants can also be used directly with just the name for package body elements. |
There was a problem hiding this comment.
| Header constants can also be used directly with just the name for package body elements. | |
| Header constants can also be referenced directly by their name inside package body elements. |
|
|
||
| case obj_package_constant: | ||
| if (!checkObjectExist(tdbb, transaction, objName, objType)) | ||
| status_exception::raise(Arg::PrivateDyn(303) << objName.toQuotedString()); // Package @1 does not exist |
There was a problem hiding this comment.
Shouldn't it be "Constant does not exist" instead ?
| if (PackageReferenceNode::constantExists(tdbb, dsqlScratch->getTransaction(), constantName)) | ||
| { | ||
| packageContext.ctx_relation = nullptr; | ||
| packageContext.ctx_procedure = nullptr; |
There was a problem hiding this comment.
These two pointers are already nullified by the dsql_ctx constructor.
| TYPE_WINDOW_CLAUSE, | ||
| TYPE_WINDOW_CLAUSE_FRAME, | ||
| TYPE_WINDOW_CLAUSE_FRAME_EXTENT, | ||
| TYPE_REFERENCE, |
There was a problem hiding this comment.
Maybe TYPE_PACKAGE_REFERENCE? Just for clarity.
| FIELD(fld_pkg_id , nam_pkg_id , dtype_long , sizeof(SLONG) , 0 , NULL , true , ODS_14_0) | ||
|
|
||
| FIELD(fld_const_name , nam_const_name , dtype_varying , MAX_SQL_IDENTIFIER_LEN , dsc_text_type_metadata , NULL , false , ODS_14_0) | ||
| FIELD(fld_const_id , nam_const_id , dtype_long , sizeof(SLONG) , 0 , NULL , true , ODS_14_0) |
There was a problem hiding this comment.
The same question as above. Since constants are always packaged and packages now have their own IDs, why would we need IDs for constants?
There was a problem hiding this comment.
Can package have more than one constant inside? In absence of ID, constants can be identified only by name which is not always convenient.
There was a problem hiding this comment.
I don't mind IDs per se, but there should be some rationale for them.
There was a problem hiding this comment.
If constants are expanded beyond packages, perhaps they will have to be presented in lock manager with their ID.
There was a problem hiding this comment.
True, but it has already been discussed and the answer was we don't need global constants.
| /* | ||
| * PROGRAM: Firebird CONSTANTS implementation. | ||
| * MODULE: Package.h | ||
| * DESCRIPTION: Routine to cache and reload Package constants |
There was a problem hiding this comment.
This module is expected to encapsulate other packaged items, so please make the description more generic (without mentioning constants).
| } | ||
|
|
||
|
|
||
| { // Execute constant expr |
There was a problem hiding this comment.
Please remove the scope as well as the redundant blank line.
| LiteralNode::genConstant(dsqlScratch, &descForDouble, false, len); | ||
| break; | ||
| } | ||
| case dtype_dec64: // dtype_dec64 is not present in LiteralNode::genConstant |
There was a problem hiding this comment.
Agreed, please add blank lines before case
| // Gen blr into dsqlScratch | ||
| Firebird::AutoMemoryPool tempPool(MemoryPool::createPool(ALLOC_ARGS0)); | ||
| CastNode cast(*tempPool, constExpr, type); | ||
| { |
There was a problem hiding this comment.
This is not the practice used in our code. Separation for readability is supported by blank lines and comments.
Package Constants
This PR adds a new database object - Package Constant (#1036). It is a constant value located in a package header (public visibility) or a package body (private visibility). See README.packages.txt for more information.
SYNTAX
Creation:
<constant_expr>is an expression that remains unchanged after recompilation.ackage constants can be queried using the expression
<package_name>.<consatnt_name>outside the package (<consatnt_name> must be a public constant) and using the expression<consatnt_name>inside the package.To query a constant, the user/role must have USAGE permission on the package.
Other SQL extensions:
Usage examples:
System Constants
As an exmaple, 3 consatnts have been added to the RDB$BLOB_UTIL package
System metadata changes
New system table - RDB$CONSTANTS
A new field has been added to
RDB$PACKAGES-RDB$PACKAGE_IDNew indexes:
Implementation
Packages have been added to the metacaching system. Since it relies heavily on identifiers, a new ID field (
RDB$PACKAGE_ID) has been added to theRDB$PACKAGEStable. Constants are stored as an array in the package metacaching object, with a package_id-to-array_id and constant_name-to-array_id mappings.There are also two important points: